Remove deprecated BlockingTrioPortal #1574
Conversation
| @@ -227,17 +213,6 @@ def _run_fn_as_system_task(cb, fn, *args, trio_token=None): | |||
| "this thread wasn't created by Trio, pass kwarg trio_token=..." | |||
| ) | |||
|
|
|||
| # TODO: This is only necessary for compatibility with BlockingTrioPortal. | |||
There was a problem hiding this comment.
I think we might still need this for the case where the user passes the trio_token?
There was a problem hiding this comment.
I have no idea, I just grepped for BlockingTrioPortal, but I'm not familiar with those APIs. @njsmith Can you please confirm?
There was a problem hiding this comment.
I think @oremanj is right. The test case would be something like:
async def test():
trio_token = trio.lowlevel.current_trio_token()
with pytest.raises(RuntimeError):
trio.from_thread.run_sync(lambda: None, trio_token=trio_token)If we fail to detect we're calling from_thread inside the Trio thread, then this will deadlock.
Codecov Report
@@ Coverage Diff @@
## master #1574 +/- ##
==========================================
- Coverage 99.69% 99.69% -0.01%
==========================================
Files 111 111
Lines 13869 13840 -29
Branches 1060 1055 -5
==========================================
- Hits 13827 13798 -29
Misses 27 27
Partials 15 15
|
|
The coverage change happens in
I think we still want to ignore names starting with |
That seems reasonable, yeah. |
|
Looks like there are a few more coverage changes still? |
We don't necessarily always have deprecated exports.
pquentin
force-pushed
the
remove-blocking-trio-portal
branch
from
fb05527 to
ddf2483
Compare
|
So close... now the last line of the new test is causing a partial coverage line, because the def not_called(): # pragma: no cover
assert False
from_thread_run_sync(not_called, trio_token=trio_token) |
Otherwise, coverage complains that we did not run the lambda.
pquentin
force-pushed
the
remove-blocking-trio-portal
branch
from
f65703e to
f5a2191
Compare
|
Thanks for investigating! This was subtle. I found about it independently, and only because I generated an HTML report that explcitly mentioned the lambda. I think passing a None functiom is simpler, and it fixes the issue too. |
|
We'll just have to switch back to passing a function when we eventually get type hints working :-) But, up to you. Feel free to merge when you're happy. |
|
Thanks! Right, I forgot about mypy, your version is better, thanks. |
TODO